fix: restrict file permissions on auth, config, and log files to prevent local credential theft by iemejia · Pull Request #244 · microsoft/fabric-cli

iemejia · 2026-06-07T12:23:24Z

Resolves [BUG] Auth, config, context, and log files created with world-readable permissions on Linux/macOS #246

Summary

Auth files (auth.json, config.json, context-{pid}.json) and debug log directories were created with default OS permissions (0o644 for files, 0o755 for directories), making them world-readable on multi-user Linux/macOS systems. This exposed MSAL token caches, auth metadata, and debug logs containing sensitive API responses to other local users.

Security Impact

File	Data at Risk	Old Permissions	New Permissions
`~/.config/fab/`	Container for all auth/config	`0o755` (world-traversable)	`0o700` (owner-only)
`~/.config/fab/auth.json`	Tenant ID, SPN client ID, identity type	`0o644` (world-readable)	`0o600` (owner-only)
`~/.config/fab/config.json`	Azure subscription IDs, capacity settings	`0o644` (world-readable)	`0o600` (owner-only)
`~/.config/fab/context-{pid}.json`	Navigation state (workspace/item names)	`0o644` (world-readable)	`0o600` (owner-only)
Log directory	Parent of debug log with HTTP traffic	`0o755` (world-traversable)	`0o700` (owner-only)

CWE-276: Incorrect Default Permissions

Changes

fab_auth.py: Write auth.json with 0o600 via os.open() instead of open()
fab_state_config.py: Create ~/.config/fab/ with mode=0o700; write config.json with 0o600 via os.open() with a new _write_restricted_file() helper
fab_logger.py: Create log directory with mode=0o700
fab_context.py: Write context-{pid}.json with 0o600 via os.open()

Cross-Platform Behavior

Linux/macOS: Fixes enforce 0o600/0o700 correctly — resolves the vulnerability
Windows: The mode parameter is a no-op (Windows uses ACLs, not POSIX permissions). Default Windows ACLs on user profile directories already restrict access to the owner, so the original behavior was not vulnerable there. These changes are safely a no-op on Windows.

Tests

6 new tests verifying:

Config directory created with 0o700
config.json written with 0o600 (initial write and overwrite)
auth.json written with 0o600 (initial write and overwrite)
Log directory created with 0o700

All existing tests pass (488 total).

…ent local credential theft Auth files (auth.json, config.json) and debug log directories were created with default OS permissions (0o644/0o755), making them world-readable on multi-user systems. This exposed MSAL token caches, auth metadata, and debug logs containing sensitive API responses to other local users. Changes: - fab_auth.py: Write auth.json with 0o600 via os.open() instead of open() - fab_state_config.py: Create ~/.config/fab/ with mode 0o700; write config.json with 0o600 via os.open() instead of open() - fab_logger.py: Create log directory with mode 0o700 - fab_context.py: Write context-{pid}.json with 0o600 via os.open() Note: On Windows the mode parameter is a no-op (Windows uses ACLs, not POSIX permissions). Default Windows ACLs on user profile directories already restrict access to the owner, so the original behavior was not vulnerable there. CWE-276: Incorrect Default Permissions

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR tightens filesystem permissions for Fabric CLI’s config/auth/context/log artifacts and adds test coverage to validate that newly created directories/files are owner-restricted.

Changes:

Restrict created directories to 0o700 (config + log directories).
Restrict created files to 0o600 by switching writes to os.open(..., mode=0o600) + os.fdopen.
Add tests asserting restricted permissions for config/auth/log paths and overwrites.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
`src/fabric_cli/core/fab_state_config.py`	Create config dir with `0o700`; write config via restricted file helper.
`src/fabric_cli/core/fab_logger.py`	Create log directory with `0o700`.
`src/fabric_cli/core/fab_context.py`	Save context file using restricted `0o600` open.
`src/fabric_cli/core/fab_auth.py`	Save auth file using restricted `0o600` open.
`tests/test_core/test_fab_state_config.py`	Add permission tests for config dir + config file.
`tests/test_core/test_fab_logger.py`	Add permission test for log directory creation.
`tests/test_core/test_fab_auth.py`	Add permission tests for `auth.json` creation + overwrite.

iemejia · 2026-06-07T12:43:30Z

+def _write_restricted_file(file_path, content):
+    """Write content to a file with owner-only permissions (0o600)."""
+    fd = os.open(file_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+    try:
+        with os.fdopen(fd, "w") as file:
+            file.write(content)
+    except BaseException:
+        # fd is already closed by os.fdopen on success; only close on failure
+        # before os.fdopen wraps it
+        raise


Fixed. _write_restricted_file() now catches Exception (not BaseException) and closes fd in the except block before re-raising. Also added _chmod_if_posix() after write to tighten pre-existing files.

iemejia · 2026-06-07T12:43:41Z

 def write_config(data):
-    with open(config_file, "w") as file:
-        json.dump(data, file, indent=4)
+    _write_restricted_file(config_file, json.dumps(data, indent=4))


Fixed. Added _chmod_if_posix(file_path, 0o600) after every write to enforce permissions on pre-existing files from older CLI versions. Also added a dedicated test (test_write_config_tightens_permissions_on_existing_file) that creates a file with 0o644 and verifies it gets tightened to 0o600 after overwrite.

iemejia · 2026-06-07T12:43:42Z

+        fd = os.open(self.auth_file, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
+        with os.fdopen(fd, "w") as file:
            file.write(json.dumps(self._get_auth_info()))


Fixed. _save_auth() now delegates to the shared _write_restricted_file() helper from fab_state_config, which handles fd-leak safety and chmod for pre-existing files. Added test_save_auth_tightens_permissions_on_existing_file to verify.

iemejia · 2026-06-07T12:43:43Z

+                fd = os.open(
+                    self._context_file,
+                    os.O_WRONLY | os.O_CREAT | os.O_TRUNC,
+                    0o600,
+                )
+                with os.fdopen(fd, "w") as f:
                    json.dump(context_data, f)


Fixed. Context file save now delegates to the shared _write_restricted_file() helper, getting both fd-leak protection and chmod for pre-existing files. Updated the context persistence tests to mock the new code path.

iemejia · 2026-06-07T12:43:45Z

 def config_location():
    _location = expanduser("~/.config/fab/")
    if not exists(_location):
-        os.makedirs(_location)
+        os.makedirs(_location, mode=0o700)
    return _location


Fixed. Changed to os.makedirs(_location, mode=0o700, exist_ok=True) and added _chmod_if_posix(_location, 0o700) to tighten pre-existing directories. Added test_config_location_tightens_permissions_on_existing_directory that creates a 0o755 dir and verifies it becomes 0o700.

iemejia · 2026-06-07T12:43:46Z

+def test_config_location_creates_directory_with_restricted_permissions(
+    monkeypatch, tmp_path
+):
+    """Verify config directory is created with mode 0o700 (owner-only access)."""
+    config_dir = tmp_path / "fab_config_test"
+    monkeypatch.setattr(
+        cfg, "config_location", lambda: _create_restricted_dir(str(config_dir))
+    )
+    # Call our helper which mimics the real config_location logic
+    location = _create_restricted_dir(str(config_dir))
+    assert os.path.isdir(location)


Fixed. Test now patches expanduser to redirect into tmp_path and calls the real cfg.config_location(). Removed unused _create_restricted_dir helper and config_dir variable. Added @pytest.mark.skipif(os.name == 'nt') to all permission assertion tests.

…s, TOCTOU fix, Windows test skip - Guard os.fdopen failures with try/except to close fd on error - Add os.chmod() after writes to tighten permissions on pre-existing files - Fix TOCTOU race in config_location() with exist_ok=True - Centralize _write_restricted_file() and reuse in auth/context modules - Skip POSIX permission assertions on Windows (os.name == 'nt') - Rewrite config_location test to exercise real function via expanduser patch - Add tests for tightening permissions on pre-existing permissive files - Update context persistence tests for _write_restricted_file usage

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

iemejia · 2026-06-07T12:43:56Z

 def _get_log_file_path():
    """Create a log file path in the user's log directory."""
    log_dir = user_log_dir("fabric-cli")
-    os.makedirs(log_dir, exist_ok=True)
+    os.makedirs(log_dir, mode=0o700, exist_ok=True)
    return os.path.join(log_dir, "fabcli_debug.log")


Fixed. Added _chmod_if_posix(log_dir, 0o700) after os.makedirs in _get_log_file_path(), matching the pattern used in config_location().

iemejia · 2026-06-07T12:43:58Z

+def _chmod_if_posix(path, mode):
+    """Best-effort chmod; no-op on Windows where POSIX modes don't apply."""
+    if os.name != "nt":
+        try:
+            os.chmod(path, mode)
+        except OSError:
+            pass


Fixed. _chmod_if_posix() now emits a logging.warning() when chmod fails on POSIX, so operators can detect restrictive filesystems or unexpected ownership conditions.

iemejia · 2026-06-07T12:43:59Z

+kind: fixed
+body: Restrict file permissions on auth, config, and log files to prevent local credential theft on multi-user systems
+time: 2026-06-07T13:40:43+02:00
+custom:
+    Author: iemejia
+    AuthorLink: https://github.com/iemejia


Fixed. Updated the changelog text to accurately describe the scope: "Restrict file and directory permissions on auth, config, context, and log paths".

iemejia · 2026-06-07T12:44:00Z


 import json
 import os
+import sys


Fixed. Removed unused sys import.

iemejia · 2026-06-07T12:44:01Z

+    monkeypatch, tmp_path
+):
+    """Verify config directory is created with mode 0o700 (owner-only access)."""
+    config_dir = str(tmp_path / ".config" / "fab")


Fixed. Removed unused config_dir variable.

…ure, test cleanup - Add _chmod_if_posix() to fab_logger.py to tighten pre-existing log directories (mirrors config_location behavior) - Emit warning via logging.warning when chmod fails on POSIX instead of silently swallowing the error - Fix changelog text to accurately describe scope (directories + files) - Remove unused sys import and config_dir variable in tests

Copilot AI review requested due to automatic review settings June 7, 2026 12:23

iemejia requested a review from a team as a code owner June 7, 2026 12:23

Copilot AI reviewed Jun 7, 2026

View reviewed changes

iemejia added 2 commits June 7, 2026 14:26

chore: add changie entry for file permissions fix

3aef7ad

Copilot AI review requested due to automatic review settings June 7, 2026 12:38

Copilot AI reviewed Jun 7, 2026

View reviewed changes

Conversation

iemejia commented Jun 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Security Impact

Changes

Cross-Platform Behavior

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

iemejia commented Jun 7, 2026 •

edited

Loading